Fix daily energy zero-bounce spikes during reconnect#314
Fix daily energy zero-bounce spikes during reconnect#314Q-Claw wants to merge 4 commits intoTypQxQ:mainfrom
Conversation
- Return unavailable when all inverter daily-energy samples are missing/invalid in a poll - Guard daily total_increasing energy sensors against transient daytime 0 drops - Keep legitimate midnight resets by allowing a reset time window
Greptile SummaryThis PR adds two complementary guards to prevent daily Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[native_value called] --> B{value_fn present?}
B -- yes --> C[Run value_fn / calculated_sensor]
C --> D{All inverter samples missing?}
D -- yes --> E[Return None\ncalculated_sensor.py]
D -- no --> F[Sum valid samples → Decimal total]
F --> G[_apply_daily_energy_zero_guard]
B -- no --> H[Read raw_value from coordinator]
H --> I[_apply_daily_energy_zero_guard]
G --> J{key in PROTECTED_DAILY_ENERGY_KEYS?}
I --> J
J -- no --> K[Return value unchanged]
J -- yes --> L{value is None?}
L -- yes --> M[Return None\nunavailable]
L -- no --> N{value_dec == 0\nAND last > 0\nAND NOT near midnight?}
N -- yes --> O[Suppress: Return None\nunavailable\nDo NOT update last_valid]
N -- no --> P[Update _last_valid_daily_energy_value\nReturn value]
style O fill:#f99,stroke:#c00
style E fill:#f99,stroke:#c00
style M fill:#fdd,stroke:#c00
Last reviewed commit: 5dbf28d |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
custom_components/sigen/sensor.py
Outdated
| def _is_near_daily_reset(self) -> bool: | ||
| """Return True around midnight to allow legitimate daily counter resets.""" | ||
| now = dt_util.now() | ||
| midnight = now.replace(hour=0, minute=0, second=0, microsecond=0) | ||
| return abs(now - midnight) <= DAILY_RESET_GUARD_WINDOW |
There was a problem hiding this comment.
the logic only handles 20 minutes AFTER midnight, not before. at 23:50 (10 min before midnight), midnight = today 00:00, now - midnight = 23h50m, which is > 20 minutes, so it returns False. this will incorrectly suppress legitimate early resets.
| def _is_near_daily_reset(self) -> bool: | |
| """Return True around midnight to allow legitimate daily counter resets.""" | |
| now = dt_util.now() | |
| midnight = now.replace(hour=0, minute=0, second=0, microsecond=0) | |
| return abs(now - midnight) <= DAILY_RESET_GUARD_WINDOW | |
| def _is_near_daily_reset(self) -> bool: | |
| """Return True around midnight to allow legitimate daily counter resets.""" | |
| now = dt_util.now() | |
| time_of_day = now.hour * 60 + now.minute | |
| # Within 20 minutes before or after midnight (23:40-00:20) | |
| return time_of_day <= 20 or time_of_day >= (24 * 60 - 20) |
Additional Comments (1)
|
- Fix symmetric midnight reset window (before + after 00:00) - Skip missing inverter daily values before safe_decimal conversion - Keep missing/invalid poll behavior as unavailable when no valid samples
|
Thanks for the review, addressed in commit Changes made:
Please re-check when convenient. |
|
@greptile-apps please re-review after commit 2ec6f00 |
custom_components/sigen/sensor.py
Outdated
| _LOGGER.warning( | ||
| "[%s] Ignoring transient daily energy drop to 0 outside reset window (last=%s)", | ||
| self.entity_id, | ||
| self._last_valid_daily_energy_value, | ||
| ) | ||
| return None |
There was a problem hiding this comment.
WARNING level causes log spam during extended outages
_LOGGER.warning(...) is called on every poll cycle while the guard is active. If polling runs every 30 seconds and an inverter is offline for an hour, this produces ~120 consecutive warning-level entries — all identical — which will visibly flood HA's logs.
Consider logging the first suppression at warning and subsequent consecutive suppressions at debug, or switching entirely to debug/info since this is expected operational behaviour (the sensor is already publishing unavailable, which provides the user-facing signal):
| _LOGGER.warning( | |
| "[%s] Ignoring transient daily energy drop to 0 outside reset window (last=%s)", | |
| self.entity_id, | |
| self._last_valid_daily_energy_value, | |
| ) | |
| return None | |
| _LOGGER.debug( | |
| "[%s] Ignoring transient daily energy drop to 0 outside reset window (last=%s)", | |
| self.entity_id, | |
| self._last_valid_daily_energy_value, | |
| ) | |
| return None |
| if valid_sample_count == 0: | ||
| _LOGGER.debug("[%s] No valid '%s' samples in this poll", log_prefix, energy_key) | ||
| return None |
There was a problem hiding this comment.
None return only covers the all-invalid case; partial-offline totals still decrease
The new valid_sample_count == 0 guard correctly prevents publishing 0 when every inverter is offline. However, consider a two-inverter setup where Inverter A goes offline (None, skipped) while Inverter B legitimately continues reporting. The function returns Inverter B's value alone — potentially a large drop from the previous combined total (e.g. 10 kWh → 5 kWh). This is less than zero (not caught by the sensor.py guard which only triggers on a drop to exactly 0), and HA's total_increasing logic may record it as a reset.
This is a pre-existing issue and this PR's scope is zero-bounce, but it's worth a comment here so the partial-offline decrease case isn't overlooked in a follow-up.
Additional Comments (1)
For most users this will be rare, but it is worth documenting (e.g. a code comment) so future maintainers understand the deliberate trade-off and are not tempted to remove or weaken the "first-poll pass-through" path. |
|
Addressed the latest review note in commit What I changed:
Validation:
@greptile-apps could you please take another look? |
|
Follow-up fix pushed in commit \ to address log-noise feedback:\n\n- Changed transient zero-guard suppression log from -> in .\n- This avoids repeated warning-level spam during extended reconnect/offline periods while preserving traceability at debug level.\n\nValidation:\n- Listing 'custom_components/sigen/sensor.py'... |
|
Follow-up fix pushed in commit
Validation:
@greptile-apps please re-review. |
| if ( | ||
| value_dec == 0 | ||
| and self._last_valid_daily_energy_value is not None | ||
| and self._last_valid_daily_energy_value > 0 | ||
| and not self._is_near_daily_reset() | ||
| ): | ||
| _LOGGER.debug( | ||
| "[%s] Ignoring transient daily energy drop to 0 outside reset window (last=%s)", | ||
| self.entity_id, | ||
| self._last_valid_daily_energy_value, | ||
| ) | ||
| return None | ||
|
|
||
| self._last_valid_daily_energy_value = value_dec | ||
| return value |
There was a problem hiding this comment.
Extended outage crossing midnight suppresses legitimate reset
If HA stays running but the Modbus/network connection drops before the pre-midnight window opens (e.g. connection goes offline at 23:39, last poll left _last_valid_daily_energy_value = 14.0), and doesn't recover until after the post-midnight window closes (e.g. 00:21), the inverter correctly reports 0 (daily counter has reset) but the guard sees:
value_dec == 0✓_last_valid_daily_energy_value = 14.0 > 0✓_is_near_daily_reset()→False(00:21 is outside the ±20 min window) ✓
The 0 is suppressed and unavailable is published instead.
This does not cause phantom energy (HA's total_increasing will handle the eventual reset correctly once production provides a non-zero reading), but it delays acknowledgment of the midnight reset and leaves daily sensors as unavailable until first real post-sunrise activity — which can look alarming in the Energy Dashboard and may interfere with automations that rely on a clean 0 at start of day.
One approach is to track the wall-clock timestamp of the last non-None reading and, if the gap exceeds a threshold (e.g. 30 min), allow the 0 through unconditionally as a probable legitimate counter reset rather than a transient glitch:
# In __init__
self._last_successful_daily_energy_ts: datetime | None = None
# In _apply_daily_energy_zero_guard, replace the suppression block:
STALE_THRESHOLD = timedelta(minutes=30)
now = dt_util.now()
stale = (
self._last_successful_daily_energy_ts is None
or (now - self._last_successful_daily_energy_ts) > STALE_THRESHOLD
)
if (
value_dec == 0
and self._last_valid_daily_energy_value is not None
and self._last_valid_daily_energy_value > 0
and not self._is_near_daily_reset()
and not stale
):
... # suppressThis keeps the glitch protection for short outages while allowing long-gap reconnects to pass a 0 through regardless.
| self._last_valid_daily_energy_value = value_dec | ||
| return value |
There was a problem hiding this comment.
_last_valid_daily_energy_value updates on value_dec == 0 during midnight window
When a midnight-window 0 passes the guard (intentionally allowed), _last_valid_daily_energy_value is set to Decimal(0). This is actually correct — once the daily counter legitimately resets, future 0 polls should not fire the guard (> 0 check prevents it). However, consider adding a short comment here to make the intent explicit, since a reader could easily assume the update should only happen for non-zero values:
| self._last_valid_daily_energy_value = value_dec | |
| return value | |
| # Update reference only for non-suppressed values (includes legitimate midnight 0 resets). | |
| self._last_valid_daily_energy_value = value_dec | |
| return value |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
This PR hardens daily
total_increasingenergy sensors against transient reconnect glitches that can cause phantom energy in the HA Energy Dashboard.Changes
custom_components/sigen/calculated_sensor.py_calculate_total_inverter_energy, track valid inverter samples per poll.None(unavailable) instead of0.0.custom_components/sigen/sensor.pyplant_daily_pv_energyplant_daily_battery_charge_energyplant_daily_battery_discharge_energyinverter_daily_pv_energyinverter_ess_daily_charge_energyinverter_ess_daily_discharge_energy>0to0outside a midnight reset window, publishNonefor that cycle.00:00.Why
Under transient Modbus/network interruptions, some daily counters can briefly report zero, which HA interprets as new production when values recover. This inflates statistics.
Result
Transient reconnect glitches become
unavailableinstead of false zero resets, preventing phantom energy spikes while preserving real midnight resets.Closes #313